feat(gcp): exclude project ids#746
Merged
Merged
Conversation
- Implemented logic to validate mutually exclusive project IDs and exclusion lists in the GCP provider. - Added support for excluding project IDs based on organization ID requirements. - Introduced new helper functions to retrieve and parse excluded project IDs from options. - Enhanced tests to cover exclusion scenarios and validation errors.
Contributor
WalkthroughAdded an organization-level GCP option Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/schema/schema.go (1)
243-267:⚠️ Potential issue | 🟠 MajorPreserve scalar
exclude_project_idsvalues during YAML unmarshalling.Line 243 routes the new key through an array-only branch. With YAML like
exclude_project_ids: "sandbox,legacy", this case writes nothing intoOptionBlock, soGetMetadata("exclude_project_ids")returns false and the exclusion/validation path never runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/schema/schema.go` around lines 243 - 267, The current branch for keys ("account_ids","exclude_account_ids","urls","services","project_ids","exclude_project_ids") only handles value as []interface{} and silently drops scalar YAML values; update the branch in the code that processes key/value (where ob is *OptionBlock, key and value are available, and valueArr/strArr are used) to also handle scalar types (string, int, float64) when value is not a slice: convert the scalar to the same string format used for array elements (including zero-pad for account_ids/exclude_account_ids), assign it to (*ob)[key] (e.g., as the single-item comma string), and keep the existing error handling for unsupported types so exclude_project_ids and similar scalar YAML values are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/providers/gcp/gcp.go`:
- Around line 214-217: The hasExclude branch currently returns a generic error
when organization_id is missing; change that to return the typed missing-key
error used elsewhere by returning &schema.ErrNoSuchKey{Name: "organization_id"}
instead of errkit.New(...). Locate the check around hasExclude and
options.GetMetadata("organization_id") in pkg/providers/gcp/gcp.go and replace
the generic error return with the typed schema.ErrNoSuchKey to make callers able
to distinguish "missing config" from other validation failures; ensure you still
use options.GetMetadata to read the config (supporting $VAR env syntax) and only
return ErrNoSuchKey when the metadata key is absent.
- Around line 814-846: The code currently leaves provider.projectScope nil when
project listing fails or excludes remove all projects, causing Resources() to
fall back to org-wide discovery; change the logic in the block that handles
getExcludeProjectIDsFromOptions(options) so that whenever enumeration fails or
the filtered projects slice is empty you assign provider.projectScope to an
explicit empty project scope (e.g., newProjectScope([]string{}) or an
emptyProjectScope sentinel) instead of leaving it nil; ensure this is done both
after applying the exclude filter and in the error path after
enrichWithProjectNumbers (use newProjectScope/emptyProjectScope and set
provider.projectScope accordingly) so that Resources() cannot revert to
organization-wide Asset API accidentally.
---
Outside diff comments:
In `@pkg/schema/schema.go`:
- Around line 243-267: The current branch for keys
("account_ids","exclude_account_ids","urls","services","project_ids","exclude_project_ids")
only handles value as []interface{} and silently drops scalar YAML values;
update the branch in the code that processes key/value (where ob is
*OptionBlock, key and value are available, and valueArr/strArr are used) to also
handle scalar types (string, int, float64) when value is not a slice: convert
the scalar to the same string format used for array elements (including zero-pad
for account_ids/exclude_account_ids), assign it to (*ob)[key] (e.g., as the
single-item comma string), and keep the existing error handling for unsupported
types so exclude_project_ids and similar scalar YAML values are preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3ca65efe-ad32-4216-9316-2bd476e74332
📒 Files selected for processing (4)
PROVIDERS.mdpkg/providers/gcp/gcp.gopkg/providers/gcp/project_scope_test.gopkg/schema/schema.go
Neo - PR Security ReviewNo security issues found Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pkg/providers/gcp/gcp.go (2)
214-217:⚠️ Potential issue | 🟡 MinorReturn a typed missing-key error for
organization_id.This path still returns a generic validation error, so callers cannot distinguish a missing required key from other config failures. Return
&schema.ErrNoSuchKey{Name: "organization_id"}here instead.As per coding guidelines, "Parse provider configuration via block.GetMetadata(key); support environment variables using $VAR syntax; return &schema.ErrNoSuchKey{Name: key} for missing required config".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/gcp/gcp.go` around lines 214 - 217, Replace the generic validation error returned when organization_id is missing with the typed schema error so callers can detect the missing key; inside the hasExclude branch where options.GetMetadata("organization_id") is checked, return &schema.ErrNoSuchKey{Name: "organization_id"} instead of errkit.New(...). Ensure the change is applied in the same conditional that references hasExclude and options.GetMetadata so callers receive the schema.ErrNoSuchKey sentinel.
816-836:⚠️ Potential issue | 🔴 CriticalFail closed when
exclude_project_idsis set but project enumeration is empty.If listing projects failed earlier, this block is skipped,
provider.projectScopestays nil, andResources()falls back toorganizations/<org>discovery. That re-includes the projects the user explicitly asked to exclude. When exclusions are configured, treat an empty or unresolved project list as an error or an explicit empty scope instead of falling back.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/providers/gcp/gcp.go` around lines 816 - 836, When exclude_project_ids (excludeIDs) is provided but the projects slice is empty you must fail instead of falling back to org discovery; add an early check after project enumeration such that if len(excludeIDs) > 0 && len(projects) == 0 you return an error (e.g., via errkit.New) indicating that no projects remain or enumeration failed, so the code in the newProjectScope/excludeScope branch is not the only path that triggers this failure; update the logic around newProjectScope/excludeScope and the existing check that returns when len(projects) == 0 so the provider does not silently revert to organizations/<org> discovery when exclusions were requested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/providers/gcp/gcp.go`:
- Around line 838-848: When configuredProjects is empty, the unscoped discovery
path currently calls manager.Projects.List() without an organization filter;
change the call that builds the project iterator to include the filter
"parent.type:organization parent.id:<organizationID>" so only projects under the
requested organization are returned. Locate the branch where configuredProjects
is checked and manager.Projects.List() is invoked (the code that later
constructs scope via newProjectScope(...) and assigns provider.projectScope),
and replace the plain List() invocation with a filtered
List().Filter(fmt.Sprintf("parent.type:organization parent.id:%s",
organizationID)) so subsequent enrichWithProjectNumbers(...) and projectScope
assignment operate only on organization-scoped projects.
---
Duplicate comments:
In `@pkg/providers/gcp/gcp.go`:
- Around line 214-217: Replace the generic validation error returned when
organization_id is missing with the typed schema error so callers can detect the
missing key; inside the hasExclude branch where
options.GetMetadata("organization_id") is checked, return
&schema.ErrNoSuchKey{Name: "organization_id"} instead of errkit.New(...). Ensure
the change is applied in the same conditional that references hasExclude and
options.GetMetadata so callers receive the schema.ErrNoSuchKey sentinel.
- Around line 816-836: When exclude_project_ids (excludeIDs) is provided but the
projects slice is empty you must fail instead of falling back to org discovery;
add an early check after project enumeration such that if len(excludeIDs) > 0 &&
len(projects) == 0 you return an error (e.g., via errkit.New) indicating that no
projects remain or enumeration failed, so the code in the
newProjectScope/excludeScope branch is not the only path that triggers this
failure; update the logic around newProjectScope/excludeScope and the existing
check that returns when len(projects) == 0 so the provider does not silently
revert to organizations/<org> discovery when exclusions were requested.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f91a2cfc-e9d0-4278-bf9e-0eaea9b7f868
📒 Files selected for processing (1)
pkg/providers/gcp/gcp.go
ShubhamRasal
requested changes
Apr 15, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/providers/gcp/gcp.go`:
- Around line 814-852: The code currently builds a per-project scope from
whatever is in the projects slice even if the Projects.List().Pages(...)
pagination failed part-way; change the logic so you only construct and assign
provider.projectScope when you know the project listing completed successfully
(or otherwise handle partial results explicitly). In practice, propagate or
check the pagination error from Projects.List().Pages(...) (or set a
"listingComplete" boolean) and if the listing was incomplete do not call
newProjectScope/enrichWithProjectNumbers/assign provider.projectScope; instead
return an error or keep global scope so Resources() does not silently skip later
pages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bc384241-3068-49f7-9422-6f6a88a23f4e
📒 Files selected for processing (1)
pkg/providers/gcp/gcp.go
ShubhamRasal
approved these changes
Apr 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
exclude_project_idsoption for GCP org-level discovery to skip specific projectsproject_ids, requiresorganization_idChanges
pkg/schema/schema.go— YAML unmarshaling forexclude_project_idspkg/providers/gcp/gcp.go— parsing, validation, exclude filtering in org providerpkg/providers/gcp/project_scope_test.go— tests for parsing, validation, filteringPROVIDERS.md— documented new option with example configTest plan
go test ./pkg/providers/gcp/ -v)go build ./...)pd-ossfrom org292477584000, confirmed 54/55 projects scanned, pd-oss absent from resultsproject_ids+exclude_project_ids→ errorexclude_project_idswithoutorganization_id→ errorSummary by CodeRabbit
New Features
exclude_project_idsparameter for GCP provider to exclude specific projects from organization-wide discovery scans. Requiresorganization_idand is mutually exclusive withproject_ids.Documentation